BalancerV3: SwapAdapter and Substreams #126
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 14904727 | Triggered | Generic High Entropy Secret | 73aec62 | evm/test/BalancerV3SwapAdapter.t.sol | View secret |
| 14904728 | Triggered | Generic High Entropy Secret | 73aec62 | evm/test/BalancerV3SwapAdapter.t.sol | View secret |
| 14904729 | Triggered | Generic High Entropy Secret | 73aec62 | evm/test/BalancerV3SwapAdapter.t.sol | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
| initialized_accounts: | ||
| - "0xbA1333333333a1BA1108E8412f11850A5C319bA9" |
There was a problem hiding this comment.
Is this the vault contract? Could you please add a comment here describing why this is necessary?
There was a problem hiding this comment.
Added comment in 302d458
ignazio-bovo
force-pushed
the
feature/balancer-v3-adapter-and-substream
branch
from
45eebca to
957f4ed
Compare
tvinagre
changed the title
tvinagre
force-pushed
the
feature/balancer-v3-adapter-and-substream
branch
4 times, most recently
from
db617f7 to
8dbc3cb
Compare
| add_change_if_accounted(&mut reserves_of, change, token.as_slice()); | ||
| } | ||
| } | ||
| if let Some(SendTo { token, .. }) = SendTo::match_and_decode(call) { |
There was a problem hiding this comment.
What happens if in the same TX the user calls SendTo and then Settle? We'd keep the final value? Also, are you sure that these are the only OPs that can alter a vault's balance?
There was a problem hiding this comment.
SendTo requires the vault to be unlocked. Doesn't it also require settle to be called in the end? If so, this call check would be redundant, but I'm not 100% sure it is a requirement
There was a problem hiding this comment.
are you sure that these are the only OPs that can alter a vault's balance?
Yes there are only 3 instances in where the _reservesOf storage mapping is being set:
settlesendToerc4626WrapOrUnwrapBuffer
There was a problem hiding this comment.
SendTo requires the vault to be unlocked. Doesn't it also require settle to be called in the end?
There are instances wheresettleandsendTocan be invoked in the same TX, this is the case with multi steps path in theBatchRouter.solcontract.
There are also instances wheresendTois invoked without asettleafter (in theRemoveLiquidityHookfor example).
The observation made however is valid: In case the storage slot for token_addr is settled twice there will be 2 corresponding token_balance messages.
W should keep the latest and most updated value according to the call order of invokation in the tx. And I have updated the HashMap insertion logic to accomodate for that.
Vault contract tokenBalance message are set according to the vault storage changes in the `_reserveOf` storage variable VaultStorage.sol contract This was the culprit that caused the failure in simulation since balancer enforces the invariant that `token.balanceOf(vault_addr) == _reservesOf[token]`
tvinagre
force-pushed
the
feature/balancer-v3-adapter-and-substream
branch
from
9c625cf to
6a49063
Compare
SSIA